Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dEdLat contribution for stress calculations is added and Universal Potentials are updated #189

Merged
merged 28 commits into from
Nov 16, 2023

Conversation

kenko911
Copy link
Contributor

@kenko911 kenko911 commented Nov 14, 2023

Summary

I finally figured out that the stress contribution due to the change of lattice vectors in the Potential class is missing in the current implementation, now I have modified the implementations to add the strain (st) variable for lattices to include the gradient of potential energy with respect to lattice vectors for stress calculations. I also added unit tests for checking force and stress implementations with central finite difference methods and completed the retraining of both universal M3GNet MLIPs. The resulting pretrained universal MLIPs achieve better accuracy and stability in benchmark examples. Finally, I have checked all Jupyter Notebook examples, and everything works for the implementation.

Checklist

  • Google format doc strings added. Check with ruff.
  • Type annotations included. Check with mypy.
  • Tests added for new features/fixes.
  • If applicable, new classes/functions/modules have duecredit @due.dcite decorators to reference relevant papers by DOI (example)

Tip: Install pre-commit hooks to auto-check types and linting before every commit:

pip install -U pre-commit
pre-commit install

kenko911 and others added 25 commits August 28, 2023 17:21
Copy link

codecov bot commented Nov 14, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (9036184) 98.00% compared to head (233cb4d) 98.59%.

Files Patch % Lines
matgl/utils/training.py 78.94% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #189      +/-   ##
==========================================
+ Coverage   98.00%   98.59%   +0.59%     
==========================================
  Files          28       28              
  Lines        1901     1928      +27     
==========================================
+ Hits         1863     1901      +38     
+ Misses         38       27      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shyuep shyuep merged commit 8ddd066 into materialsvirtuallab:main Nov 16, 2023
@J-You
Copy link

J-You commented Sep 21, 2024

I'm checking out the source in VS Code, and I jumped in from this merged PR. Not sure if this is the right place to ask, but I've got a question about model/_m3gnet.py. On lines 341 and 342, when converting from fractional to Cartesian coordinates, why is it that the matrix product only involves the first row vector of the lattice?

g.edata["pbc_offshift"] = torch.matmul(g.edata["pbc_offset"], lat[0])
g.ndata["pos"] = g.ndata["frac_coords"] @ lat[0]

If so, then how does the subsequent code operate, such as the function compute_pair_vector_and_distance:

dst_pos = g.ndata["pos"][g.edges()[1]] + g.edata["pbc_offshift"]
src_pos = g.ndata["pos"][g.edges()[0]]
bond_vec = dst_pos - src_pos
bond_dist = torch.norm(bond_vec, dim=1)

Jiang You

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants